[PATCH] QtQml: Do not clear objects' propertyCaches on last GC run

The property caches are not specific to the engine. The same object may
be exposed to other engines and still require its property cache. When
the clearing of the property caches on engine destruction was
introduced, the property caches were still engine-specific and we had no
choice but to clear them. Otherwise any further access would lead to a
dereference of a dangling pointer.

Furthermore, when clearing the JS wrapper for a QObject, check if it's
actually the wrapper being deleted. We don't want to clear some other
engine's wrapper.

Amends commit 749a7212e903d8e8c6f256edb1836b9449cc7fe1.
Amends commit c6b2dd879d02b21b18f80faab541f8f04286685a.

Fixes: QTBUG-142514
Change-Id: I40bb1aeca65225d56cb1d2ff498f5f1722216a70
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit dc2358e98b8ddab532866a403ffc09d1162ad0f9)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit abd054a491b5f443193033d451dc17e09841277a)

Gbp-Pq: Name upstream_qtqml_crash_fix.diff

diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index e1a5ed1f2e082e484c61bb178fd320e466ddcc95..6f3758e18cfd242274a40d2d8e5f6acbc7a971e3 100644 (file)
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -1583,10 +1583,9 @@ void QObjectWrapper::destroyObject(bool lastCall)
                     o->deleteLater();
             } else {
                 // If the object is C++-owned, we still have to release the weak reference we have
-                // to it.
-                ddata->jsWrapper.clear();
-                if (lastCall && ddata->propertyCache)
-                    ddata->propertyCache.reset();
+                // to it. If the "main" wrapper is not ours, we should leave it alone, though.
+                if (ddata->jsWrapper.as<QObjectWrapper>() == this)
+                    ddata->jsWrapper.clear();
             }
         }
     }

diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
index 2f855213bae52d633930ff1bd3ea162b3620c630..4c1e7e5a86b81a97ea1ddd34bd40ff50c12c1288 100644 (file)
--- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp
+++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
@@ -1116,7 +1116,7 @@ void tst_QJSEngine::newQObjectPropertyCache()
         engine.newQObject(obj.data());
         QVERIFY(QQmlData::get(obj.data())->propertyCache);
     }
-    QVERIFY(!QQmlData::get(obj.data())->propertyCache);
+    QVERIFY(QQmlData::get(obj.data())->propertyCache);
 }
 
 void tst_QJSEngine::newQMetaObject() {

diff --git a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
index 67cdefa30d9380b15e104ea950eb750af282f2a3..6983a372175efd8411d0e2b611a093b9aafa2a8c 100644 (file)
--- a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
+++ b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt
@@ -324,6 +324,7 @@ set(qml_files
     stringLength.qml
     stringToByteArray.qml
     structuredValueType.qml
+    multiEnginePropertyCache.qml
     takenumber.qml
     testlogger.js
     text.qml

diff --git a/tests/auto/qml/qmlcppcodegen/data/multiEnginePropertyCache.qml b/tests/auto/qml/qmlcppcodegen/data/multiEnginePropertyCache.qml
new file mode 100644 (file)
index 0000000..222b2f8
--- /dev/null
+++ b/tests/auto/qml/qmlcppcodegen/data/multiEnginePropertyCache.qml
@@ -0,0 +1,20 @@
+pragma Strict
+import QtQml
+
+QtObject {
+    id: root
+
+    property int foo: 0
+    onFooChanged: root.close1()
+
+    property int bar: 0
+    onBarChanged: close2()
+
+    function close1() {
+        console.log("close1")
+    }
+
+    function close2() {
+        console.log("close2")
+    }
+}

diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
index 4038376897c3fe83707e8c8f18ceff33a36997d0..368eb1fe8a36a36f9c69941b88bfde362c404a56 100644 (file)
--- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
+++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
@@ -279,6 +279,7 @@ private slots:
     void stringLength();
     void stringToByteArray();
     void structuredValueType();
+    void multiEnginePropertyCache();
     void takeNumbers();
     void takeNumbers_data();
     void testIsnan();
@@ -5777,6 +5778,26 @@ void tst_QmlCppCodegen::structuredValueType()
     QCOMPARE(o->property("w2").value<WeatherModelUrl>(), w2);
 }
 
+void tst_QmlCppCodegen::multiEnginePropertyCache()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, QUrl(u"qrc:/qt/qml/TestTypes/multiEnginePropertyCache.qml"_s));
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    std::unique_ptr<QObject> o(c.create());
+    QVERIFY(o);
+
+    {
+        QJSEngine other;
+        other.newQObject(o.get());
+    }
+
+    QTest::ignoreMessage(QtDebugMsg, "close1");
+    o->setProperty("foo", 1);
+
+    QTest::ignoreMessage(QtDebugMsg, "close2");
+    o->setProperty("bar", 2);
+}
+
 void tst_QmlCppCodegen::takeNumbers()
 {
     QFETCH(QByteArray, method);